-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: show spinner on exports #15107
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15107 +/- ##
==========================================
- Coverage 77.54% 77.35% -0.19%
==========================================
Files 967 968 +1
Lines 49752 49880 +128
Branches 6351 6374 +23
==========================================
+ Hits 38578 38583 +5
- Misses 10972 11095 +123
Partials 202 202
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
fd05030
to
c1a3dd9
Compare
superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM I just had the one nit about type checking.
* feat: show spinner on exports * Set cookie only if token is passed * Use iframe * Small fixes * Fix lint * Remove stale test * Add explicit type
* feat: show spinner on exports * Set cookie only if token is passed * Use iframe * Small fixes * Fix lint * Remove stale test * Add explicit type
* feat: show spinner on exports * Set cookie only if token is passed * Use iframe * Small fixes * Fix lint * Remove stale test * Add explicit type
SUMMARY
Show a spinner while an export is being generated.
This is tricky because currently when the user clicks "export" we immediately redirect them to
/api/v1/database/export/?q=${rison.encode([database.id])}
usingwindow.assign
. So while we can set a spinner, we don't know when the download is generated to stop the spinner.With this PR, in order to stop the spinner the frontend generates a short ID, which is passed to the backend with the export request. Once the export is ready, the backend will set a cookie with the short ID as name. The frontend is constantly polling for that cookie, and when it finds the cookie it hides the spinner.
One problem with this solution is that the GIF stops animating while the export is being generated, since the animation runs on the main thread. To keep the GIF animated the export code injects an iframe, and loads the export there. When the cookie indicating that the download is ready is found, the iframe is deleted.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before, no spinner.
After:
export_spinner.mov
Note: I added an artificial 5 seconds delay in generating the exports for the video, so the spinner can be seen.
TESTING INSTRUCTIONS
Tested with all exports (see screencast). Also tested with the non-card view, though not shown in the video.
ADDITIONAL INFORMATION